- 
                Notifications
    You must be signed in to change notification settings 
- Fork 6
Refactor handling of Jinja2 templates and add filters #327
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| Codecov ReportAttention: Patch coverage is  @@             Coverage Diff             @@
##           develop     #327      +/-   ##
===========================================
+ Coverage    71.46%   72.45%   +0.99%     
===========================================
  Files           87       91       +4     
  Lines         7990     8165     +175     
  Branches      1544     1572      +28     
===========================================
+ Hits          5710     5916     +206     
+ Misses        1875     1838      -37     
- Partials       405      411       +6     
 Flags with carried forward coverage won't be shown. Click here to find out more. 
 ... and 4 files with indirect coverage changes 🚀 New features to boost your workflow:
 | 
c9d7233    to
    b29559e      
    Compare
  
    78400a5    to
    cb7816b      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be great to point to that reference page from the main infrahub documentation, I'm thinking about (might be some other places):
- Jinja2 computed attributes
- Jinja2 transform (artifact)
| --- | ||
|  | ||
|  | ||
| ## Builtin Jinja2 filters | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe worth pointing to the doc on this one (https://jinja.palletsprojects.com/en/stable/templates/#list-of-builtin-filters) so users know what's behind each line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added these links.
cb7816b    to
    66ac470      
    Compare
  
    
Refactors the management of Jinja2 templates, some notes on this PR.
templateparameter as either astror apathlib.Pathobject, in the first case the template will be rendered from a string and the latter part will use file based templates.get_variables()method will return a list of any variables that are required by the template, a current limitation for now is that for file based templates this is only supported for the first file and not if the template includes other files as imports (this part can be added later, I just want to have it in place for the string based computed attributes and it's not used anywhere today)Note that we talked about providing a prefix for the netutils filters, though I'm not sure this is required now that we've listed the source of each filter within the documentation. Any thoughts around that?